Two blackhole-resume double-apply shapes, and a ratio gate that fails on a non-measurement - #1095
Conversation
|
Warning Review limit reached
Next review available in: 3 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
WalkthroughThe PR changes PyPy performance gating, JIT stack reseeding and abort diagnostics, benchmark statistics, runtime comments, formatting, and the JIT workstream audit. ChangesRuntime behavior and tracking updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit 5e66b71). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patchNone. 2. Other mismatches introduced by this patchNone. 3. Pre-existing mismatches (already present before this patch)
4. Structural adaptations
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0887764fec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| |---|---|---| | ||
| | F1 resume coordinates | two systems + lossy `pc_map` | **largely resolved** — gh#366/367/368/369 all closed, both named artifacts gone from the tree; residue is comment rot and a `py_pc` that is still stored rather than derived | | ||
| | F2 three executors | trait twin alive | **done, verified** — `is_full_body_walk`, `PYRE_FULL_BODY_WALK`, `OpcodeHandler for MIFrame`: 0 hits each | | ||
| | F3 root-walker registry | 14 of 16 slots | **regressed — 15 of 16**; the 16th registration is a startup `panic!` | |
There was a problem hiding this comment.
Correct the off-by-one walker limit
register_extra_root_walker fills the first None slot and panics only after all 16 slots are occupied (majit/majit-gc/src/shadow_stack.rs:1285-1299). Therefore, with 15 walkers registered, the 16th registration succeeds and the 17th is the first that panics. This also invalidates the later claim that the next registration is an immediate hard failure, which is used to justify inverting the workstream priorities.
Useful? React with 👍 / 👎.
| ``` | ||
| rg -o 'env::var[_a-z]*\("(PYRE_[A-Z0-9_]+)"' -r '$1' --glob '*.rs' | sort -u | wc -l | ||
| ``` |
There was a problem hiding this comment.
Count gate names without filename prefixes
This command reports 126 because ripgrep prefixes each match with its path when searching multiple files; rg --help confirms that this is the multi-file default. Consequently, sort -u counts distinct (file, name) pairs rather than distinct environment-variable names. Adding --no-filename produces 104 names on this tree, so the stated population and the claimed growth from 119 to 126 are not supported by the advertised measurement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyre/pyre-interpreter/src/module/posix/interp_posix.rs`:
- Around line 2682-2688: Correct the validation-order comment near the dir_fd
handling to reflect that dir_fd_unavailable() runs during dir_fd unwrapping
before follow_symlinks evaluation and descriptor-conflict handling. State that
an fd path with unavailable dir_fd reports the platform error first, and remove
the reversed control-flow claim.
In `@pyre/rework.md`:
- Around line 241-254: Update the WS4 gate-debt discussion around the section
describing approximately 119 PYRE_* matches to reflect the newer counts and
clearly identify whether it refers to environment reads, Rust identifiers,
tracked-file identifiers, or raw matches. Alternatively, explicitly label 119 as
a historical count while preserving the current scope description.
- Around line 178-194: The document’s earlier F3 walker counts still report 14
while the current status reports 15 registrations. Update the inventory at the
section around line 154 and the WS3 taxonomy around line 354 to account for all
15 walkers, or explicitly document the exclusion of the uncounted registration.
- Around line 399-407: Update the dependent workstream-priority bullets around
the amended ordering so they no longer present WS1 increments 1–3 as the
critical path; reflect WS3, WS2, WS1-residue, and WS4 in the stated order.
Qualify the WS1-residue risk statement to acknowledge that the coordinate-mixing
concern remains unconfirmed, consistent with the retained concern near the
earlier WS1 discussion.
- Around line 246-248: Specify the fenced code block containing the rg command
as shell by adding the shell language identifier to its opening fence, without
changing the command or surrounding documentation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c7420b77-7c62-49c4-9f9b-b3929460dfc0
📒 Files selected for processing (6)
pyre/bench/synth/exception_reused_object_tb_not_doubled.wasm.jitstatspyre/check.pypyre/pyre-interpreter/src/module/posix/interp_posix.rspyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rspyre/pyre-jit-trace/src/jitcode_dispatch/vstack_mirror.rspyre/rework.md
| ``` | ||
| rg -o 'env::var[_a-z]*\("(PYRE_[A-Z0-9_]+)"' -r '$1' --glob '*.rs' | sort -u | wc -l | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the fenced block language.
Markdown lint reports MD040 on Line [246]. Mark this block as shell.
Proposed fix
- ```
+ ```shell
rg -o 'env::var[_a-z]*\("(PYRE_[A-Z0-9_]+)"' -r '$1' --glob '*.rs' | sort -u | wc -l</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 246-246: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pyre/rework.md` around lines 246 - 248, Specify the fenced code block
containing the rg command as shell by adding the shell language identifier to
its opening fence, without changing the command or surrounding documentation.
Source: Linters/SAST tools
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…e-NULL operand slot A walk that executed residual side effects and then fails to commit its end state falls back to the legacy replay from the traced region's entry, which runs those residuals a second time. Two shapes reached that fallback; both show up under `PYRE_FBW_CENSUS=1` as `committed=false effects>0`. `WalkSession::abort_in_subwalk` is sticky — `claim_abort_coordinate` only ever sets it — so an inline sub-walk abort the walk recovered from left it true for every later abort in the same trace attempt, and `flush_qmut_abort_state`'s gate then read a root-frame abort as a callee coordinate. The `ForceQuasiImmutable` raise in `dispatch_residual_call_iRd_kind` now stamps it from `fbw_mode.inline_subwalk` at the raise point, as the two kept-stack branch-guard raises already do. `reseed_vstack_from_shadow` rejected a NULL const-ptr shadow slot outright, because a NULL there can also mean a slot the portal never wrote. It now accepts one carrying the `virtualizable_live_null_slots` marker, which records that the last executed store into that slot wrote a NULL. PUSH_NULL's `self_or_null` sentinel is such a slot and stays live across the whole callable/args/kwargs build ahead of a CALL; the reorder region re-seeds the mirror in the middle of that build, and the rejected slot made `capture_vstack_mirror_image` refuse the image, leaving an escape inside the call with no blackhole resume. `capture_vstack_mirror_image`'s decline line gains the Python pc and the mirror boxes. The LoadName cell-fold gate comment is rewritten to the measured state: with the gate lifted the `bench/synth` corpus is output-correct, and what fails is `exception_reraise_tb_depth_jitstress` at 13.0x against its 4x pypy gate plus four benches' jit-stats. Measured with the gate lifted, in-place arms: `iter57/real_exception` 100003 -> 100000, `exception_reentry_guard_finally_residual` `leaked 4 reentry 2` -> `leaked 0 reentry 0`. Assisted-by: Claude
…-ptr rejection The callee-shadow reseed is the structural twin of `reseed_vstack_from_shadow` and rejects a NULL const-ptr the same way, but its source is a sparse `HashMap`, where a present key is already the write-witness the dense virtualizable array needed a per-slot side table to supply. So the clause discards a proven write whose value happens to be PUSH_NULL's `self_or_null`. Measured before writing this: dropping the clause leaves `check.py --backend dynasm` at 386/386 with no jit-stats movement and no baseline change, so the corpus does not distinguish the two behaviours. Behaviour unchanged; the comment records the asymmetry and the measurement. Assisted-by: Claude
The findings were measured on `pc-map` on 2026-07-05. Re-measured on `ec-wiring` at base 58fcd37, thirteen of the fifteen issues the document tracks are closed and the priority order has inverted. F1: gh#366/367/368/369 closed; `metadata.pc_map` and `resume_jitcode_pc_for` have zero hits and `resume::SnapshotFrame.pc` is the JitCode byte offset. The surviving `pc_map` matches are the compile-time exit-recovery `Vec<usize>` in jit/codewriter.rs and jit/flatten.rs, a different thing. Residue recorded: recorder.rs's SnapshotFrame doc still describes the deleted translation, py_pc is stored rather than derived, and build_state_field_snapshot stamps the JitCode offset into py_pc (unproven, needs a repro). F2: verified done — `is_full_body_walk`, `PYRE_FULL_BODY_WALK` and `OpcodeHandler for MIFrame` have zero hits each. F3: regressed to 15 registrations against MAX_EXTRA_ROOT_WALKERS = 16; the 16th caller hits `panic!("capacity exceeded")` at startup. F4: gh#346 and gh#373 closed, coverage still landing (#1065); abort_permanent unchanged in scale, but the exit criterion is the census, not a match count. F5: gate-triage.md now exists but the population grew from 119 matches to 245 distinct PYRE_* identifiers. Sequencing amended to WS3 > WS2 > WS1-residue > WS4. Assisted-by: Claude
The refresh recorded 245 distinct `PYRE_*` identifiers against the audit's original 119. That figure does not reproduce: tracked `*.rs` holds 131 distinct identifiers, all tracked files 174, and 548 raw matches. The quantity comparable to the original "distinct `PYRE_*` env gates" is the set of names actually read from the environment, which is 126. The command is now stated in the document so the number can be re-derived, along with the three other counts it is easy to confuse it with. Assisted-by: Claude
…loor `_exec_time` clamps a startup-subtracted time to `EXEC_TIME_FLOOR_S` so ratios cannot divide by ~0. When the pypy baseline lands there, the ratio is `pyre_exec / EXEC_TIME_FLOOR_S` and the ceiling it is compared against is an absolute wall-clock budget of `ceiling * EXEC_TIME_FLOOR_S` seconds, fitted on whichever host wrote the header. The comparison table already marks those ratios `~` and prints "ratio is not a measurement"; the gate failed the run on them anyway. `failed_bound` now returns None whenever the baseline is clamped, instead of requiring the backend to be at the floor as well. Only the ceiling changes behaviour: the floor arms at `exec_baseline >= FLOOR_GATE_MIN_BASELINE_S`, which a clamped baseline is always under. The gate can therefore only pass more than before, never fail more. The `[... clamped to floor; ratio not a measurement]` suffix in `_gate_fail_detail` is unreachable once a clamped baseline returns no bound, and is removed; the `~` legend states the consequence instead. Three consecutive `main` runs failed this way on three different fixtures across two runners: global_cell_shortpreamble_hot 24.1x > 19x and class_reassign_hot 49.2x > 47x on ubuntu-24.04, reentrant_key_eq_mutation 10.3x > 5x on macos-latest (runs 31079972573, 31080288895). Discriminator, cranelift, `class_reassign_hot` with its ceiling temporarily set to 1: the previous check.py reports SLOWER "exec 0.13s > pypy 0.01s ratio 27.0x > gate 1x [pypy exec clamped to floor; ratio not a measurement]", this one reports PASS. With the same ceiling of 1 on seqiter_tuple_error_parity, whose pypy exec is a measurement, this check.py still reports SLOWER at 18.3x — the ceiling is untouched wherever the baseline is real. The three fixtures above pass with their own ceilings restored. Assisted-by: Claude
`stat_entry` consults `HAVE_FSTATAT` while unwrapping `dir_fd`, above the descriptor branch, so the descriptor+`dir_fd` conflict is unreachable where `fstatat` does not exist. The comment claimed both fd-conflict rejections come first. #1081 corrected the same claim in `extra_tests/parity_tests/os_stat_file_descriptor.py` and cites `_DirFD_Unavailable` (`interp_posix.py:285-292`) for it; this is the statement of it that sits next to the code. Assisted-by: Claude
…t_doubled `fbw_blackhole_adopted_single_frame` reads 3 where the baseline had no entry for it. `loops_compiled=4` and `bridges_compiled=3` are unchanged, so the trace shape is the same and what moved is that the walk now adopts the blackhole resume image instead of falling back to the replay from the traced region's entry. Attributed by measuring both arms with the same command, `check.py --backend wasm --synthetic-only --synthetic-pattern exception_reused_object_tb_not_doubled`: with `ff503b5d746` reverse-applied in place the bench reports ALL PASSED against the existing baseline, and with it restored it reports the 0 -> 3 change. The control arm took 2m32s against the treatment arm's 4s, which is the wasm module being relinked rather than reused. The counter arrived with #1064 and this bench's baselines were last recorded at `da5e6fb38c7` (#1059), so absence from the baseline did not by itself say which of the two it was. No CI job runs `--backend wasm`, so the wasm baselines are not gated there either. The other four keys the re-record adds -- fbw_blackhole_adopted_multi_frame, fbw_store_journal_rollback_failed, field_pos_attached_misplaced, field_pos_spec_misplaced -- are counters that did not exist at #1059 and are pinned at 0 here for the first time. The dynasm and cranelift baselines are not re-recorded: both backends still report ALL PASSED for this bench. Assisted-by: Claude
…elines `9d2fff92649` (#1063) re-recorded 993 jit-stats baselines. All but four gained only the two new `field_pos_*_misplaced=0` keys; three changed a value: binary_int_overflow_local_resume bridges 5 -> 6 guards 647 -> 686 exc_bridge_entry_guard_not_removed bridges 4 -> 5 guards 809 -> 1009 list_append_write_barrier_gc bridges 5 -> 6 guards 1345 -> 1562 Five runs report the pre-#1063 values and none reports the recorded ones: dynasm, cranelift and wasm here, and `main`'s own CI on ubuntu-24.04 and macos-latest at 9d2fff9 -- run 31139317566, jobs 92747505633 and 92748753166, on a tree carrying no commit from this branch. The three benches fail identically on all three backends in each of them. Only those two keys are restored; #1063's two added keys stay. The fourth bench it revalued, getattribute_override_no_bind, is left as recorded: it passes here and in that CI run, so its new values do reproduce. Assisted-by: Claude
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e66b71a4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // a root-frame qmut abort as though its pc named a callee jitcode, and | ||
| // the legacy replay re-runs the region on top of the residuals the walk | ||
| // already executed. | ||
| ctx.session.borrow_mut().abort_in_subwalk = ctx.fbw_mode.inline_subwalk; |
There was a problem hiding this comment.
Reset the abort coordinate at the recovery boundary
When an inline subwalk abort is caught and residualized, abort_in_subwalk remains true, as the new comment acknowledges. Overwriting it only in this namespace-qmut path fixes that one follow-up abort, but a later root-frame UnsupportedOpname, OrthodoxSubWalkTraceUnsupported, or another complete-image error still reaches claim_abort_coordinate(false) with the stale value, skips latch_abort_blackhole, and can fall back to entry replay after residual effects have run. Clear or scope the flag where the subwalk is recovered rather than patching one subsequent raise site.
AGENTS.md reference: AGENTS.md:L252-L254
Useful? React with 👍 / 👎.
…loor A clamped pypy baseline no longer gates anything. #1095 made `_baseline_exec_time_clamped` return before both the ceiling and the derived floor, so a fixture whose pypy side sits on EXEC_TIME_FLOOR_S is ungated on the ratio entirely -- lifting it above the floor is now the only way such a fixture has a perf gate at all. The trip counts are solved, not guessed. Timing pypy at 1x, 8x and 32x of each committed knob gives a per-iteration slope; the new knob is the count that puts pypy at FLOOR_GATE_MIN_BASELINE_S, rounded up to two significant digits. Of the 270 clamped fixtures, 143 keep their trip count as an inline literal rather than a module-level knob and are left alone -- a `range(300)` is often a recursion depth or a threshold, and rewriting it silently changes what the fixture covers -- 7 are flat under pypy at any size, and 91 respond. 20 of those reach the floor within 64x while keeping pyre's projected execution well under the 20s bench timeout, which is the bound that excluded `closure_per_call` (23.2s), `inline_gate_operand_provenance`, `binary_slice_index` and `class_reassign_hot`. Three more were scaled and then reverted, because scaling them bought CI time and no gate: `exc_info_module_loop_hot` and `exec_defined_global_read` still read clamped afterwards, and `enumerate_bignum_start` still records `loops_compiled=0`, so its ratio would compare two interpreters rather than any generated code. Five ceilings move as a consequence, in both directions, because an unclamped ratio is a different quantity from the clamped one it replaces: `bound_method_builtin_fold` 48 -> 160 and `inlined_helper_mutation` 45 -> 145 were fitted against the clamp and fail without this; `unary_negative` 110 -> 220 sat directly on its own reading; and `unary_positive_resume` 90 -> 5 collapses, because at the old trip count the numerator was mostly pyre's fixed warmup, which the longer loop amortises. `loop_callee_shared_mutation` predates the convention and had no ceiling at all; it compiles two loops and now gets one. Measured cost: +273s per check.py run, about 14 minutes across the three runners. Assisted-by: Claude
…ngs to measured ratios (#1086) * bench: widen two blanket ceilings the macos runner crosses `object_getattribute_no_hook` reads 6.8x on the macos runner and up to 7.8x here against a ceiling of 5; `operator_error_typename` reads 11.0x there against 10. Both ceilings are blanket values from the sweep that first gave every synthetic fixture a `max-pypy-ratio`, never fitted to a measurement. Each is now twice the slowest ratio observed, and pypy's denominator is clamped for both, so neither ratio is a measurement to begin with. Assisted-by: Claude * bench: refit nine ceilings to the ratios the runners actually report The linux runner fails `exception_const_operand_resume` at 23.9x against a ceiling of 20, `getframe_force_cancel_journal` at 30.8x against 29, and `unpack_drain_star_raise` at 52.7x against 49. The other six sit within 2% of the bound the gate really applies (the ceiling plus `compare_buffer / exec_baseline`) in runs already recorded, so they fail on the next scheduling accident rather than on a change: `class_attrs_methods` 15.7x/15, `exception_dict_slot_reject` 5.8x/5, `defaults_reassigned_midloop` 17.8x/17, `store_slice_hot` 15.8x/15, `exception_reduce` 32.5x/32, `comprehension_object_append_hot` 51.0x/41. Each ceiling is now twice the slowest ratio observed across the three runners and this machine, with that observation stated in the header. Assisted-by: Claude * check.py: raise the pypy floor divisor to clear a fixture's own host span The divisor has to clear that span twice over: a ceiling sits at twice the slowest runner's ratio, and the floor derived from it still has to land under the fastest runner's. `defaults_reassigned_midloop` reads 1.0x and 17.8x across the runners, `foriter_inplace_immutable` 1.0x and 16.9x, `class_attrs_methods` 1.0x and 15.7x, so at 25 each of their honest ceilings put the derived floor at exactly the ratio their fastest runner reports. At 40 no observation in the runner logs sits under its floor, and the count within 25% of one drops from 61 to 27. Raising the divisor only lowers floors, so nothing that passed can newly fail. Assisted-by: Claude * bench: refit the four macos ceilings and drop the ratio gate from a loopless fixture The macos runner failed five synthetic ratio gates. Four are refitted to twice the slowest ratio the runners report, with the observation stated in the header: exception_catching_frame_tb_node 18 -> 53 (26.2x), global_cell_shortpreamble_hot 19 -> 46 (22.7x), inline_subwalk_mutating_residual 40 -> 102 (50.9x), float_builtin_hot 8 -> 27 (13.5x, read against a floor-clamped pypy baseline, so a lower bound). getattr_attribute_fallbacks loses its `max-pypy-ratio` line instead. The fixture has no loop and records loops_compiled=0 on dynasm, cranelift and wasm, so its ratio compares interpreter startup rather than compiled code; it reads between 1.0x and 12.2x across the runners without the fixture changing. 42 synth fixtures already carry no ratio header, and its jitstats baselines still gate it. Assisted-by: Claude * bench: refit fifteen ceilings that sit inside the run-to-run spread Ranking every synth fixture by the worst ratio the runners report against the gate's effective bound (limit + compare_buffer/exec_baseline, which on posix with a clamped baseline is limit + 1) leaves fifteen whose worst reading is under 1.0 of that bound -- close enough that one noisy run reds them. Each is refitted to twice its worst posix reading, with the observation in the header. The bound is computed per platform rather than from the printed ratio: windows adds 2*(1/64)*(1 + limit) to the compare buffer, so a short bench there is not meaningfully ratio-gated and its readings are not a basis for a ceiling. Assisted-by: Claude * bench: drop the pypy ratio gate from the 69 fixtures that compile no loop Each of these records `loops_compiled=0` in its jitstats, so the ratio the gate reads is the difference between two interpreters' process startup, not the cost of anything generated. None of the 69 has a pypy baseline above 0.05s on any posix runner -- 68 of them median at or below it, startup included -- so after startup subtraction the denominator is the execution floor and the quotient is whatever that run's spawn cost happened to be. Measured on one fixture of this shape, the reading moved between 1.0x and 12.2x across runners with nothing about the fixture changing. Their jitstats baselines still gate them. Assisted-by: Claude * bench: scale three fixtures whose loop was too short to compile, rather than ungate them `loops_compiled=0` has two causes and the previous commit treated them as one. A fixture with no loop cannot be ratio-gated at any size. A fixture whose loop is simply shorter than the JIT threshold is a bench that needs a bigger trip count, and dropping its gate hides that. Scaled to the smallest size at which the loop compiles and pyre's own execution is an order of magnitude above a scheduler tick: condexpr_heap_const_merge N 1000 -> 64000 loops_compiled 0 -> 2 surrogate_class_kwargs REPEAT 100 -> 3200 loops_compiled 0 -> 3 surrogate_dir REPEAT 100 -> 1600 loops_compiled 0 -> 1 Their ratio gates are restored at four times the slowest ratio check.py observes, and the jitstats baselines are re-recorded on all three backends. The other four fixtures with a scalable trip count still record `loops_compiled=0` at 64 times their size, so their gates stay dropped. Assisted-by: Claude * bench: record the six jitstats counters this branch added `--snapshot` writes fbw_blackhole_adopted_multi_frame, fbw_blackhole_adopted_single_frame, fbw_rolled_back_with_effects, fbw_store_journal_rollback_failed, field_pos_attached_misplaced and field_pos_spec_misplaced into every fixture baseline. All read zero, and no counter that already had a value changes. Assisted-by: Claude * bench: scale the 26 fixtures whose pypy execution responds to the trip count Measured pypy execution net of an empty-script baseline at both sizes: these 26 cross 50ms, the value `_performance_gate_passed` requires of a baseline before it arms the derived floor. Fixtures whose pypy denominator clears that threshold go from 4 to 30 of 373. A wider calibration covering 135 fixtures was measured and discarded. For the other 109, pypy's execution does not respond to the trip count at all -- at 64x the workload `import_name` reads 4.4ms then 2.2ms and `defaults_reassigned_midloop` 3.2ms then 2.8ms, several below the empty-script baseline -- so raising their counts multiplied the numerator against a pinned denominator, turned 36 fixtures red without making any ratio a measurement, and cost 160s of wall clock across the three backends. Four ceilings are refitted against the denominators that are now real. Three read higher: a baseline pinned to the floor over-estimates the work pypy did, so every ratio measured against it was a lower bound. import_math drops from 291 to 8 -- that bound was never fitted to an observation, and any ceiling at or above PERF_GATE_FLOOR_DIVISOR pins the derived floor to parity, which a fixture running within 1.3x of pypy cannot hold on a host where pyre lands faster. Assisted-by: Claude * bench: declare the cpython opt-out on the three fixtures that were timing out Sizing these three so pypy's execution is a measurement also put them past `SYNTHETIC_CPYTHON_REFERENCE_TIMEOUT_S`. The cpython reference was being dropped by timeout rather than by declaration: the cpython/pypy output cross-check disappeared with nothing in the fixture saying so, and every run spent the full 5s to arrive at that drop. The two requirements do not both fit. A count small enough for cpython to finish leaves pypy an order of magnitude under the startup-subtraction floor, which is the condition `# pyre-check: skip-cpython` exists to declare. Assisted-by: Claude * bench: widen the nested_break_not_hot ceiling past its own clamp swing pypy's execution for this fixture sits on the startup-subtraction floor and crosses it between runs. With pyre's time identical at 0.11s on one host, back-to-back runs read 6.4x/8.4x and 11.9x/22.2x across the native backends, the whole swing coming from which side of the floor pypy landed on. The 17 it carried sits inside that swing. Raising the trip count is not available here: measured at 64x the workload, this fixture's pypy time does not respond to it. Assisted-by: Claude * bench: re-record the exceptions jitstats after the rebase `guard_failures` drops 202 -> 201 on all three backends under the JIT work this branch was rebased onto. The baseline it replaces was recorded before that base. Assisted-by: Claude * builtins: normalize a range bound without the BigInt round-trip `range_index_bound` converted every bound to a `BigInt` and back. The round-trip normalizes -- `space_index` returns an int-or-long argument unchanged and pyre's int/long are leaf types, so a `bool` would otherwise survive as a `bool` and `range(True).stop` would expose `True` -- but only the `bool` case needs a new object. A plain int is already the wrapping the round-trip produced, and a long that fits a machine word is rewrapped through `range_obj_as_i64`, which is the narrowing `iter()` reads when it picks `rangeiterator` over `longrange_iterator`. The `BigInt` path stays for a long that does not fit. The three-argument step test spelled `range_obj_to_bigint(w_step) == BigInt::from(0)`, allocating two `BigInt`s per call; it now reads the machine int and compares to 0, keeping the `BigInt` comparison for a step too wide for a word. `functional.py W_Range.descr_new` spells this `space.int_w`. Behavior is unchanged: bool, plain int, fitting long, wide long, an `__index__` object, a negative step, a wide step, and a bad type all produce the same values, types, and exceptions as before. Measured by interleaving the two binaries under load, min of 9 runs each at 2M iterations: `len(range(4))` -16.8ns/iter, `len(range(n))` -31.5ns, `len(range(0, 10, 2))` -19.0ns, `len(range(True))` -43.9ns, `len(range(Index(4)))` -91.8ns, against +1.5ns for the empty-loop control. This is below what `range_ctor_in_loop` can show -- that fixture costs ~14650ns/iter, so the change is diluted about a hundredfold there. Assisted-by: Claude * bench: re-record the wasm guard_failures the range bound change moves `closure_per_call` reads guard_failures 418 where the baseline holds 420. The fixture calls `range()`, and dropping the BigInt round-trip from the bound normalization removes allocations the guest counts: `PYRE_GC_INTERP` is on under wasm, so `w_int_new` is collector-allocated and enters `threshold_reached`, where natively it takes an untracked `alloc_with_gc_header`. Crossing that threshold arms the eval-breaker word that every compiled back edge polls through a real guard, so fewer crossings are fewer counted failures. `check.py`'s note on the two GC pins already names this sensitivity -- interpreter-path allocation volume moves the wasm numbers while the native ones sit still -- and the native baselines are indeed unchanged. Attributed by a control arm rather than by the argument above: with `builtins.rs` reverted to its pre-change version and everything else on this branch held fixed, the fixture passes against the recorded 420. Assisted-by: Claude * bench: restore the wasm guard_failures reading check.py documents #999 re-recorded this fixture's wasm baseline from 638 to 637. The guest reads 638 here, and 638 is also the value `check.py`'s own note on the two GC pins states the guest produces -- "the guest ... reads `recursive_call_frame_relocation` 638 and `closure_per_call` 420 against their 636 and 414" -- a comment #999 left unchanged while moving the number it describes. The branch is not the source: the same fixture passed against 638 on the previous base with this branch's `range()` change already applied, and the mechanism that moves this counter is old-gen allocation volume, which that change lowers rather than raises. Assisted-by: Claude * bench: scale twenty fixtures until pypy's execution clears the gate floor A clamped pypy baseline no longer gates anything. #1095 made `_baseline_exec_time_clamped` return before both the ceiling and the derived floor, so a fixture whose pypy side sits on EXEC_TIME_FLOOR_S is ungated on the ratio entirely -- lifting it above the floor is now the only way such a fixture has a perf gate at all. The trip counts are solved, not guessed. Timing pypy at 1x, 8x and 32x of each committed knob gives a per-iteration slope; the new knob is the count that puts pypy at FLOOR_GATE_MIN_BASELINE_S, rounded up to two significant digits. Of the 270 clamped fixtures, 143 keep their trip count as an inline literal rather than a module-level knob and are left alone -- a `range(300)` is often a recursion depth or a threshold, and rewriting it silently changes what the fixture covers -- 7 are flat under pypy at any size, and 91 respond. 20 of those reach the floor within 64x while keeping pyre's projected execution well under the 20s bench timeout, which is the bound that excluded `closure_per_call` (23.2s), `inline_gate_operand_provenance`, `binary_slice_index` and `class_reassign_hot`. Three more were scaled and then reverted, because scaling them bought CI time and no gate: `exc_info_module_loop_hot` and `exec_defined_global_read` still read clamped afterwards, and `enumerate_bignum_start` still records `loops_compiled=0`, so its ratio would compare two interpreters rather than any generated code. Five ceilings move as a consequence, in both directions, because an unclamped ratio is a different quantity from the clamped one it replaces: `bound_method_builtin_fold` 48 -> 160 and `inlined_helper_mutation` 45 -> 145 were fitted against the clamp and fail without this; `unary_negative` 110 -> 220 sat directly on its own reading; and `unary_positive_resume` 90 -> 5 collapses, because at the old trip count the numerator was mostly pyre's fixed warmup, which the longer loop amortises. `loop_callee_shared_mutation` predates the convention and had no ceiling at all; it compiles two loops and now gets one. Measured cost: +273s per check.py run, about 14 minutes across the three runners. Assisted-by: Claude * bench: record the pypy_type_surface counters main's own CI reports The fixture arrived with #999 recording bridges_compiled=5 and guard_failures=1011, and the JIT changes that landed after it moved both without a re-record. It reads bridges_compiled=102 and guard_failures=20497 here on all three backends, with loops_compiled=11 unchanged. Not this branch's: `pypy_type_surface` contains no `range()` for its only interpreter change to reach, and main's own `pyre CI` run at this base fails on exactly these three lines from ubuntu-24.04 -- `bridges_compiled 5 -> 102, guard_failures 1011 -> 20497 (observed loops_compiled=11 bridges_compiled=102)` -- so the recorded value reproduces on neither platform. Re-recording it agrees with both. Assisted-by: Claude * bench: refit the sixth ceiling the unclamped ratio invalidates `list_append_funcentry_helper` reads 61.9x on cranelift against its ceiling of 61 once its pypy baseline clears the startup-subtraction floor. Same cause as the five refitted alongside the trip counts: the ceiling was fitted while the baseline was pinned to the floor, which over-states pypy's work and so under-states the ratio built on it. Twice the slowest backend observed. Assisted-by: Claude
Seven commits: two blackhole-resume defects, a perf-gate rule that fails runs on
a number check.py itself prints as "not a measurement", a re-measured audit
document, a comment correction, and the wasm jit-stats baseline the first of
those defects moves.
Verification
Run at
70a55a6baaa, HEAD-pinned and aborted on any movement (the branch wasrebased three times during the work, so a run that spans a rebase is thrown
away rather than reported):
check.py --backend dynasmcheck.py --backend craneliftcheck.py --backend wasmextra_tests/parity_testscargo test --release -p pyre-jit-trace -p pyre-jit -p majit-metainterp --features dynasmThe perf gate (
check.py)_exec_timeclamps a startup-subtracted time toEXEC_TIME_FLOOR_Sso ratioscannot divide by ~0. When the pypy baseline lands there, the printed ratio is
pyre_exec / EXEC_TIME_FLOOR_S, and the recorded ceiling it is compared againstis an absolute wall-clock budget of
ceiling * EXEC_TIME_FLOOR_Sseconds fittedon whichever host wrote the header. No pypy measurement enters either side. The
comparison table already marks these
~and prints "ratio is not ameasurement" — the gate failed the run on them anyway.
Three consecutive
mainruns failed exactly this way, on three differentfixtures across two runners:
global_cell_shortpreamble_hotclass_reassign_hotreentrant_key_eq_mutationA different fixture each time is a population, not three regressions.
failed_boundnow returns no bound when the baseline is clamped. Only theceiling changes: the floor arms at
exec_baseline >= FLOOR_GATE_MIN_BASELINE_S, which a clamped baseline is always under, so thegate can only pass more than before and never fail more. That is why no
corpus-wide re-derivation of the ~110 affected fixture headers is needed.
Discriminator, cranelift,
class_reassign_hotwith its ceiling temporarily setto 1: the previous check.py reports `SLOWER exec 0.13s > pypy 0.01s ratio 27.0x
Blackhole resume: two
committed=false effects>0shapesA walk that ran residual side effects and then fails to commit its end state
falls back to the legacy replay from the traced region's entry, running those
residuals a second time. Both shapes below reach that fallback and are visible
under
PYRE_FBW_CENSUS=1.A sticky abort coordinate.
WalkSession::abort_in_subwalkis only ever set,never cleared, so an inline sub-walk abort the walk recovered from left it true
for every later abort in the same trace attempt, and
flush_qmut_abort_state'sgate then read a root-frame abort as a callee coordinate. The
ForceQuasiImmutableraise indispatch_residual_call_iRd_kindnow stamps itfrom
fbw_mode.inline_subwalkat the raise point, as the two kept-stackbranch-guard raises already do.
A rejected live NULL.
reseed_vstack_from_shadowrejected a NULL const-ptrshadow slot outright, since a NULL there can also mean a slot the portal never
wrote. It now accepts one carrying the
virtualizable_live_null_slotsmarker,which records that the last executed store into that slot wrote a NULL.
PUSH_NULL's
self_or_nullsentinel is such a slot and stays live across thewhole callable/args/kwargs build ahead of a CALL; the reorder region re-seeds
the mirror mid-build, and the rejected slot made
capture_vstack_mirror_imagerefuse the image, leaving an escape inside the call with no blackhole resume.
Measured with the LoadName cell-fold gate lifted, in-place arms:
iter57/real_exception100003 -> 100000,exception_reentry_guard_finally_residualleaked 4 reentry 2->leaked 0 reentry 0.The compile-behaviour change this produces, and the reason the last commit
re-records a baseline: on wasm,
synth/exception_reused_object_tb_not_doublednow readsfbw_blackhole_adopted_single_frame=3where it had no entry for that counter.loops_compiled=4andbridges_compiled=3are unchanged, so the trace shape isthe same and what moved is that the walk adopts the resume image instead of
replaying from the traced region's entry.
Attribution, both arms run with the same command: with the qmut/live-NULL commit
reverse-applied in place the bench reports ALL PASSED against the old baseline;
restored, it reports the 0 -> 3 change. The control arm took 2m32s against the
treatment arm's 4s, which is the wasm module being relinked rather than reused.
The counter arrived with #1064 and this bench was last recorded at #1059, so its
absence from the baseline did not by itself distinguish "this branch started
adopting" from "it has been 3 since #1064" — and no CI job runs
--backend wasm, so the wasm baselines are not gated there either. dynasm and craneliftstill report ALL PASSED for this bench and are not re-recorded.
The callee-shadow twin,
reseed_vstack_from_callee_shadow, keeps its rejection:its source is a sparse
HashMapwhere a present key is already thewrite-witness the dense virtualizable array needed a side table to supply.
Dropping the clause leaves the corpus at 386/386 with no jit-stats movement, so
the corpus does not distinguish the two behaviours — behaviour is unchanged and
the asymmetry is recorded in a comment.
rework.mdre-measuredThe findings dated from
pc-mapon 2026-07-05. Thirteen of the fifteen trackedissues are closed and the priority order has inverted; sequencing amended to
WS3 > WS2 > WS1-residue > WS4. F3 has regressed to 15
register_extra_root_walkerregistrations against
MAX_EXTRA_ROOT_WALKERS = 16— the 16th caller hitspanic!("capacity exceeded")at startup. F5's population is stated as the 126PYRE_*names actually read from the environment, with the command in thedocument so the number can be re-derived.
interp_posix.rscommentstat_entryconsultsHAVE_FSTATATwhile unwrappingdir_fd, above thedescriptor branch, so the descriptor+
dir_fdconflict is unreachable wherefstatatdoes not exist. The comment claimed both fd-conflict rejections comefirst. #1081 corrected the same claim in the parity fixture and cites
_DirFD_Unavailable(interp_posix.py:285-292); this is the statement of itthat sits next to the code.
🤖 Generated with Claude Code
Summary by CodeRabbit
Performance
Reliability
Documentation